-
Notifications
You must be signed in to change notification settings - Fork 637
feat(tools): add EthProofValidator #10026
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
feat(tools): add EthProofValidator #10026
Conversation
…ion with Rust-based ZK verifiers
| // The following include Single-GPU (1:100) zk proofs | ||
| // var blockIds = new List<long> { 24046700, 24046800, 24046900, 24047000, 24047100, 24047200, 24047300, 24047400, 24047500, 24047600, 24047700 }; | ||
|
|
||
| const long LatestBlockId = 24080984; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why hardcode these?
|
|
||
| const long LatestBlockId = 24080984; | ||
| const int BlockCount = 25; | ||
| var blockIds = Enumerable.Range((int)(LatestBlockId - BlockCount), BlockCount+1).Select(i => i).ToList(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: generally use explicit types instead of var for clarity
| var results = await Task.WhenAll(tasks); | ||
|
|
||
| int validCount = results.Count(r => r == 1); | ||
| int totalCount = results.Count(r => r != -1); // Exclude skipped proofs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no need to iterate twice here
| int totalCount = results.Count(r => r != -1); // Exclude skipped proofs | ||
|
|
||
| Console.WriteLine(" -----------------------------"); | ||
| Console.WriteLine((double)validCount / totalCount >= 0.5 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: could use int arithmetic
|
|
||
| private async Task<int> ProcessProofAsync(ProofMetadata proof, ZkProofVerifier? verifier) | ||
| { | ||
| if (verifier == null) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| if (verifier == null) | |
| if (verifier is null) |
| Console.WriteLine($" Proof {proof.ProofId,-10} : {(isValid ? "✅ Valid" : "❌ Invalid")} ({verifier.ZkType}, {sw.ElapsedMilliseconds} ms)"); | ||
| return isValid ? 1 : 0; | ||
| } | ||
| catch (Exception ex) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
catch specific exception type
| : $"❌ BLOCK #{blockId} REJECTED ({validCount}/{totalCount})"); | ||
| } | ||
|
|
||
| private async Task<int> ProcessProofAsync(ProofMetadata proof, ZkProofVerifier? verifier) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
returning enum type seems clearer than int here
| { | ||
| return await _httpClient.GetFromJsonAsync<List<ClusterVerifier>>("/api/v0/verification-keys/active"); | ||
| } | ||
| catch (Exception ex) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
catch specific exception
|
|
||
| public ZkProofVerifier? GetVerifier(string clusterId) | ||
| { | ||
| _verifiers.TryGetValue(clusterId, out var verifier); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
handle case where not found in dictionary?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the method is intentionally designed to return null to indicate a cache miss. This allows the caller (i.e. BlockValidator) to handle the missing verifier by calling TryAddVerifierAsync, which performs the lazy-loading of verification keys from the API
…cessing with detailed results and error handling
…pdate Dockerfile paths
rubo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall, good. Just small things here and there.
…gisterVerifier method
rubo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks good to go, just some minor things. The only concern for now is the Rust build on each .NET build.
| int zk_type, | ||
| [In] byte[] proof_ptr, // Pins automatically for the duration of call | ||
| nuint proof_len, | ||
| IntPtr vk_ptr, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Prefer nint over legacy IntPtr
| IntPtr vk_ptr, | |
| nint vk_ptr, |
| public class ZkProofVerifier : IDisposable | ||
| { | ||
| private readonly ZKType _zkType; | ||
| private IntPtr _vkPtr; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| private IntPtr _vkPtr; | |
| private nint _vkPtr; |
| if (_vkPtr != IntPtr.Zero) | ||
| { | ||
| Marshal.FreeHGlobal(_vkPtr); | ||
| _vkPtr = IntPtr.Zero; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| if (_vkPtr != IntPtr.Zero) | |
| { | |
| Marshal.FreeHGlobal(_vkPtr); | |
| _vkPtr = IntPtr.Zero; | |
| if (_vkPtr != nint.Zero) | |
| { | |
| Marshal.FreeHGlobal(_vkPtr); | |
| _vkPtr = nint.Zero; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not much of a fan of binary files in the repo, though these two are small. What do you think about having them base64- or hex-encoded instead?
| namespace Nethermind.EthProofValidator; | ||
|
|
||
| class Program | ||
| { | ||
| static async Task Main(string[] args) | ||
| { | ||
| if (args.Length < 2 || !int.TryParse(args[0], out int latestBlockId) || !int.TryParse(args[1], out int blockCount)) | ||
| { | ||
| Console.WriteLine("Usage: dotnet run <LatestBlockId> <BlockCount>"); | ||
| Console.WriteLine("Example: dotnet run 24182425 25"); | ||
| return; | ||
| } | ||
|
|
||
| BlockValidator validator = new BlockValidator(); | ||
|
|
||
| while (blockCount-- > 0) | ||
| { | ||
| int blockId = latestBlockId - blockCount; | ||
| var sw = System.Diagnostics.Stopwatch.StartNew(); | ||
| await validator.ValidateBlockAsync(blockId); | ||
| sw.Stop(); | ||
| Console.WriteLine($"⏱️ Total Time: {sw.ElapsedMilliseconds} ms"); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can convert this to top-level app to simplify a bit:
| namespace Nethermind.EthProofValidator; | |
| class Program | |
| { | |
| static async Task Main(string[] args) | |
| { | |
| if (args.Length < 2 || !int.TryParse(args[0], out int latestBlockId) || !int.TryParse(args[1], out int blockCount)) | |
| { | |
| Console.WriteLine("Usage: dotnet run <LatestBlockId> <BlockCount>"); | |
| Console.WriteLine("Example: dotnet run 24182425 25"); | |
| return; | |
| } | |
| BlockValidator validator = new BlockValidator(); | |
| while (blockCount-- > 0) | |
| { | |
| int blockId = latestBlockId - blockCount; | |
| var sw = System.Diagnostics.Stopwatch.StartNew(); | |
| await validator.ValidateBlockAsync(blockId); | |
| sw.Stop(); | |
| Console.WriteLine($"⏱️ Total Time: {sw.ElapsedMilliseconds} ms"); | |
| } | |
| } | |
| } | |
| using Nethermind.EthProofValidator; | |
| if (args.Length < 2 || !int.TryParse(args[0], out int latestBlockId) || !int.TryParse(args[1], out int blockCount)) | |
| { | |
| Console.WriteLine("Usage: dotnet run <LatestBlockId> <BlockCount>"); | |
| Console.WriteLine("Example: dotnet run 24182425 25"); | |
| return; | |
| } | |
| BlockValidator validator = new BlockValidator(); | |
| while (blockCount-- > 0) | |
| { | |
| int blockId = latestBlockId - blockCount; | |
| var sw = System.Diagnostics.Stopwatch.StartNew(); | |
| await validator.ValidateBlockAsync(blockId); | |
| sw.Stop(); | |
| Console.WriteLine($"⏱️ Total Time: {sw.ElapsedMilliseconds} ms"); | |
| } | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After that, if you're gonna use command-line options, it's better to use a dedicated library. We use System.CommandLine. For example, see the other projects in tools, like DocGen or SendBlobs.
| <Target Name="BuildRustDependency" BeforeTargets="Build"> | ||
| <Exec Command="cargo build --release" WorkingDirectory="../native-zk-verifier" /> | ||
| <ItemGroup> | ||
| <NativeLibs Include="../native-zk-verifier/target/release/libnative_zk_verifier.so" Condition="$([MSBuild]::IsOSPlatform('Linux'))" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We usually use a bit more sophisticated approach with the runtimes folder, but maybe this is enough for now.
BTW, this rebuilds the Rust lib whenever you change the .NET code. I'm not sure we want this. Maybe we can have libraries pre-built and committed to Git LFS. WDYT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I found a solution to configure the .csproj to only trigger cargo build if the Rust source files actually change. This solves the re-build problem, but the downside is that developers still need the Rust toolchain installed.
If strict independence from Rust is the priority, I can switch to the Git LFS / pre-built binary approach you suggested. The trade-off there is that it replaces the source code with only the binary, so we lose the ability to check the native rust code implementation
Let me know which trade-off you prefer!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then let's go with the former approach. If Rust toolchain becomes an issue, then we'll reconsider.
|
Also, added a build check in ea40923 |
…base64 encoded data for airbender verifier
…uments in block validation
…nly when `native-zk-verifier` code changes
| <Target Name="BuildRustDependency" BeforeTargets="Build"> | ||
| <Exec Command="cargo build --release" WorkingDirectory="../native-zk-verifier" /> | ||
| <ItemGroup> | ||
| <NativeLibs Include="../native-zk-verifier/target/release/libnative_zk_verifier.so" Condition="$([MSBuild]::IsOSPlatform('Linux'))" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then let's go with the former approach. If Rust toolchain becomes an issue, then we'll reconsider.
Changes
This PR introduces an initial approach for a proof of concept (PoC) tool,
EthProofValidator, located insrc/tools. This tool is designed to validate Ethereum block proofs aggregated by https://ethproofs.org/Key additions:
EthProofValidator): Orchestrates the fetching of proofs from the Ethproofs API and acts as a wrapper for verification.native-zk-verifier): A native library that implements the actual cryptographic verification logic using various zkVM SDKs (SP1, Pico, OpenVM, Zisk, Airbender).P/Invoketo bridge the .NET application with the Rust-based verifiers.Workflow:
ethproofs.org.Types of changes
What types of changes does your code introduce?
Testing
Requires testing
Documentation
Requires documentation update
A
README.mdhas been included within thetools/EthProofValidatordirectory explaining the project structure and build/run instructions.Requires explanation in Release Notes
Remarks
native-zk-verifierlibrary.EthProofValidator.csprojautomatically detects the OS (Linux/Windows/OSX) to load the correct shared library extension (.so,.dll,.dylib).